Repository navigation
Wake idle Bots for unfinished conversation tasks - #1115
Conversation
A board write starts a continue turn for each owner's next task, so a group assignment advances every member in the same pass. One Bot still does one task at a time. An open turn is not woken again, and a task waiting on an answer does not loop. Fixes #1109 Co-authored-by: Danyang Chen <birfychen@gmail.com>
|
Verdict: PASS at 83e4111 Ran:
Not run:
Acceptance (#1109, including the owner note)Checked the diff and
Docs: Risks
Gaps (non-blocking)
Security fence / display names: not in scope. The cue strips newlines from the title and caps it at 200 characters. The wake is host text, not a remote untrusted body. Role No actionable defect against the #1109 acceptance. This comment is not merge authority and does not enqueue. |
…lowLab#1110) (ThinkFlowLab#1137) Owner: Dev3 Reviewer: Reviewer Base: fc19e6d Tip SHA: b214c11 Fixes ThinkFlowLab#1110 Replayed onto `fc19e6d2` (it was `781121ba`): `ThinkFlowLab#1148` landed the author attestation in `review-gate.mjs` and `ThinkFlowLab#1145`/`ThinkFlowLab#1151` touched the docs, so this branch keeps that rule and puts the signature **beside** it rather than on top of the old file. The two files that switch the gate on — `.github/agent/signers.json` and the `signers` roster in `review-policy.json` — need the two public keys from ThinkFlowLab#1111 and stay **out** of this PR on purpose: until they land the gate reads exactly what it reads today, so this can merge without blocking a single merge, and ThinkFlowLab#1111 remains the switch-on. ## Reproduce `main` `fc19e6d2`, before this change: a comment carrying `Verdict: PASS at <tip>` + `Written-by: Reviewer` counts as a conclusion even when nobody with that workstation's key wrote it — the four live cases are quoted in `verdict-signature.test.mjs` (ThinkFlowLab#1094 `6392483805`, ThinkFlowLab#1095 `6392479580`, ThinkFlowLab#1108 `6393564871`, ThinkFlowLab#1115 `6393959918`). Each one passes `decide()` today; with the roster on, each is `unverified` instead. ## What changed - `.github/scripts/verdict-signature.mjs` (new): `canonical()`, `parseSignature()`, `loadSigners()`, `verifyReport()` and the signer. Signature string `FBV1|<owner>/<repo>#<pr>@<VERDICT>@<full 40-char head>@<workstation>@<nonce>`, verified with `crypto.verify(null, …, { key, format: 'der', type: 'spki' })`. Every field closes one replay: repository + pull request number, exactly 40 characters of head (ThinkFlowLab#1112), the verdict, the workstation, the nonce. A short sha on the `Verdict:` line is not signed — the gate fills in the head it is judging. - `review-gate.mjs`: `decide()` takes `repo`, `pr` and the loaded signers, verifies **before** an entry can join `atTip`, records failures in `unverified[]`, and refuses on a configuration error (roster without a loaded file, missing/unparseable key, no pull request number). `render()` prints the unverified block. - `merge-gate.mjs` / `enqueue-guard.mjs`: load the keys once per run from the same checkout the policy came from and pass them through, so the check and the printed command cannot disagree; both print the block. - `.github/scripts/sign-verdict.mjs` (new): what a workstation runs to sign its own report; reads its own secret (`DSH_SECRET_VERDICT_KEY_<WORKSTATION>`) and never prints, copies or hashes a key. - Docs move with the protocol: `AGENTS.md` (Merge gates, ThinkFlowLab#627), `docs/autonomous-dev.md` (Signed conclusions), `.claude/skills/review-pr` (the report format). One changelog fragment. ## What I ran - `node --test ".github/scripts/*.test.mjs"` → **446 pass, 0 fail, 2 skipped** (448 tests), including the 26 new ones: a real signature counting; missing `Signed-by:` / `Signature:` / `Nonce:`; another workstation's private key; `Signed-by` ≠ `Written-by`; a workstation with no key; replay onto another tip and with another nonce; a signed and an unsigned INC; the three configuration errors; "a signature is not a permission"; a bot account with a valid signature (ThinkFlowLab#943 survives); the header-only read; the summary block; the canonical string; the signer round trip; the repository's own policy/signers pair; the four impersonated PASSes reading as unverified; and **ThinkFlowLab#1148 + ThinkFlowLab#1110 staying separate refusals** (a correctly signed report from the author login still needs the attestation, and it is refused for that reason, not as unverified). - `node scripts/ds-bot-patches.mjs check` → two `ok` lines (no `core/` file is touched). - `node .github/scripts/changelog.mjs check` → `57 fragment(s) … well formed`. ## Red without the source change - `review-gate.mjs` back to `fc19e6d2` → `verdict-signature.test.mjs` **7 pass / 19 fail** of 26. - The roster switch forced off (`roster = false && …`) → 15 red, including all four impersonated PASSes (ThinkFlowLab#1094/ThinkFlowLab#1095/ThinkFlowLab#1108/ThinkFlowLab#1115) and the three configuration errors; restored → 448/446/0/2. ## Not run - `core/` and every plugin suite: the diff touches `.github/`, `docs/` and `changelog.d/` only — `ds-bot-patches check` confirms `core/` equals `ds-bot/` + patches. - `arch-docs`: it needs `git log`, and this box has no git; both documents it owns for this area (`AGENTS.md`, `docs/autonomous-dev.md`) are updated here. - `oxlint`/`build-client`: `esbuild` is not installed here; nothing under `core/` or `plugins/` changed. ## Risk - Behaviour change only once the roster exists (ThinkFlowLab#1111). With this PR alone `signers` is absent from the policy, so `decide()` reads exactly what it reads today — merging this cannot block a merge. - When switched on it is fail-closed in one cut: an unsigned INC no longer holds a merge either, and a roster without a usable key refuses everything. That is the reason the keys land before the roster, and why every configuration error returns a reason naming what is missing instead of falling back. - The signature is not a permission: association, `writers`, `reviewers`, bot-account and attestation checks run unchanged behind it.
…ands alone (ThinkFlowLab#1392) fix(issue-ref): read a bare ThinkFlowLab#1234 only when the number stands alone Fixes ThinkFlowLab#1391 Owner: Dev2 Reviewer: Reviewer Written-by: Dev-2 Tip SHA: `0a0887327804685aae28a977fde7a26d0833f894` Base: `10884b8b` (`main`, `ThinkFlowLab#1385`) Tree: `cfee808d8b6e9deeac63b16c54abb5b8ea92274c` ## What changed `core/src/shared/issue-ref.js` guarded only the character *after* the digits (`#(\d{1,7})(?![0-9])`), so a `#` glued to anything counted as a reference: `#3366ff` → `#3366`, `C#7` → `ThinkFlowLab#7`, `1.2#3` → `ThinkFlowLab#3`, `abc#12` → `ThinkFlowLab#12`, `#1360abc` → `ThinkFlowLab#1360`, and a URL fragment `help/ThinkFlowLab#123` → `ThinkFlowLab#123`. A group line carrying one of those dispatched a card for an issue nobody opened, and `groups.js` empties `speakers` once a card is written, so the line got no answer. The matcher now keeps three separate shapes. - A **bare** `#N` counts only with a separator in front — `(?<![\w./-])`, so a word character, `.`, `-` or `/` glues it to something else — and a non-word character behind (`(?![\w])`, which also refuses `#1360abc`). - **`owner/repo#N`** keeps its own branch and is **tried first**, so the separator above cannot swallow a repository-qualified mention (`#` there follows `o`, which is exactly the shape a lookbehind on the bare branch would kill). - **`github.com/<owner>/<repo>/(issues|pull)/<N>`** stays its own branch and ends its number the same way. `MENTION` (`cardMentions`, and `withoutMentions` in `work-dispatch.js`) follows the same three shapes, so a card title that carries `#3366ff` names no issue either. `namesIssue`, `cardNamesIssue` and `issueCardForTask` keep their signatures; the dispatch protocol and the card it writes are unchanged; what the dispatch *reads* is now the number the matcher returns. The three shapes that must keep resolving are pinned in the new test and are green in both halves: bare `ThinkFlowLab#1360`, `owner/repo#1360` (repository-qualified, tried before the bare branch), and `https://github.com/o/r/issues/1360`. `core/src/host/work-dispatch.js` took the id from the raw match (`'#' + (match[1] ?? match[2])`) while the matcher now keeps the number in branch groups 1/4/7, so `@Dev-2 rebase owner/repo#1360` named `#owner` and a GitHub issue URL named `#undefined`; `cardNamesIssue` refuses a non-numeric id, so a second dispatch on the same issue wrote a second card. Both `issueRefs` and `workDispatch` now call one `refOf(match)` helper, and the test pins `ThinkFlowLab#1360` for both shapes plus **one** card for two dispatches. **Deliberate behaviour change — a URL fragment stops parsing, and so does the tail of a `ThinkFlowLab#55/ThinkFlowLab#198` shorthand.** `…/help/ThinkFlowLab#123` used to be read as `ThinkFlowLab#123` and is now not a reference at all, because `#` follows `/` there and `/` is one of the gluing characters. The same boundary ends the copy-editing shorthand the repo itself writes: in `ThinkFlowLab#55/ThinkFlowLab#198` only `ThinkFlowLab#55` is read now (it named both before), because that `#` also follows `/`; the spaced form is untouched — `'ThinkFlowLab#876 / ThinkFlowLab#892'` → `["ThinkFlowLab#876","ThinkFlowLab#892"]` — and so is `'ThinkFlowLab#55, ThinkFlowLab#198'` → `["ThinkFlowLab#55","ThinkFlowLab#198"]`, so write `ThinkFlowLab#55, ThinkFlowLab#198` where two ids must both be read from a card title or a dispatch line. Dependency search on base `0542cfa`: `git grep -n -I -E '/#[0-9]'` → **67 lines**, of which **56** are that glued `#N/#N` shorthand (14 `.github/**`, 14 `docs/**`, 12 `plugins/**`, 4 `core/src`, 3 `core/test`, 2 root `*.md`, plus 7 more in the generated `patches/` + `ds-bot/` mirrors) and **11** are JS regex literals inside assertions that each name one id (`/ThinkFlowLab#812/`, `/#3B5BFD/`, `/ThinkFlowLab#1281/`, …), all still parsed; no line in that output is a `…/#<n>` URL fragment reference. Two readers, not one: `.github/**` never imports `issue-ref.js` — `enqueue-rules.mjs` carries its own `const ISSUE_REF = /^[\s>#-]*(?:fixes|closes)…/im` (`:30`), and `merge-gate.mjs` / `enqueue-guard.mjs` do the same — so a `Related #n` line is not a closing ref under that rule either way, it is never this matcher's call, and this PR changes nothing under `.github/`. No test imports `core/src/shared/issue-ref.js`; the only consumer under test is `core/test/work-dispatch.test.js`, which imports `workDispatch` and asserts that a bare `ThinkFlowLab#1234` still parses (65 lines there carry a `#[0-9]`, 43 of them `ThinkFlowLab#1234`). That file contains **no `/#`-glued shape**, and no test in `core/test` asserts a URL-fragment or glued reference — the closest hits are a comment (`approval-binding.test.js:5`, `ThinkFlowLab#49/ThinkFlowLab#48`) and colour literals (`branding.test.js:40-41`, `/#3B5BFD/`). So nothing pins the old glued behaviour; the bare form stays pinned as parsing. The prose that does use the shorthand — `docs/subsystems/core-host.md:267` (`ThinkFlowLab#55/ThinkFlowLab#198`), `docs/autonomous-dev.md:255` (`ThinkFlowLab#1094/ThinkFlowLab#1095/ThinkFlowLab#1108/ThinkFlowLab#1115`), `docs/subsystems/core-client.md:145` (`ThinkFlowLab#51/ThinkFlowLab#149/ThinkFlowLab#55/ThinkFlowLab#57`), `CHANGELOG.md:389` (`ThinkFlowLab#499/ThinkFlowLab#504`), `AGENTS.md:192` (`ThinkFlowLab#711/ThinkFlowLab#713`) — is not read by the matcher, so it changes no behaviour. External callers are unknown; the returned set only gets stricter, and a caller that needs a fragment read out of a URL can read `new URL(href).hash` itself. ## Red first, same command in both halves `cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/issue-ref.test.js` Half (a): this tree with the new test but the old `issue-ref.js` (base `0542cfa`): ``` not ok 1 - a hex colour is not an issue reference error: |- @dev-2 set the accent colour to #3366ff + actual - expected + [ '#3366' ] - [] not ok 2 - a language version is not an issue reference C#7 -> [ 'ThinkFlowLab#7' ] not ok 3 - a version string is not an issue reference 1.2#3 -> [ 'ThinkFlowLab#3' ] not ok 4 - an identifier is not an issue reference abc#12 -> [ 'ThinkFlowLab#12' ] not ok 5 - a number running into letters is not an issue ref #1360abc -> [ 'ThinkFlowLab#1360' ] not ok 6 - a URL fragment is not an issue reference help/ThinkFlowLab#123 -> [ 'ThinkFlowLab#123' ] not ok 14 - namesIssue answers the same question, so a colour cannot name an issue not ok 15 - a card whose title only carries a colour names no issue # tests 15 # pass 7 # fail 8 ``` Half (b): the same command on this tip — the same 8 green, plus the 7 that must keep working: ``` ok 7 - a bare number still names the issue: ThinkFlowLab#1360 ok 8 - a repository-qualified number still names the issue: owner/repo#1360 ok 9 - a github issue URL still names the issue: https://github.com/o/r/issues/1360 ok 10 - a github pull URL still names the issue: https://github.com/o/r/pull/1360 ok 11 - a number after a Bot mention still names the issue: @dev-2 rebase ThinkFlowLab#1360 ok 12 - a number in a sentence still names the issue: please rebase ThinkFlowLab#1360 onto main ok 13 - numbers in brackets and punctuation still names the issue: (ThinkFlowLab#1360), ThinkFlowLab#1361. # tests 15 # pass 15 # fail 0 ``` ### The dispatch consumer, same command, two states `cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/work-dispatch.test.js` At this tip (`0a088732`): **19/19 green**. With `core/src/shared/issue-ref.js` and `core/src/host/work-dispatch.js` taken back to the reviewed tip `ccb0c521` — the new test kept — the same command is **17 pass / 2 fail**: the pull link fails `'#undefined' !== 'ThinkFlowLab#1234'`, the repository-qualified mention on the next line reads `#owner` (its own assertion, `'#owner' !== 'ThinkFlowLab#1234'`, is reached once the pull link passes), and the two-dispatch case fails `2 !== 1`. ``` not ok 1 - the dispatch shape is an issue id and an imperative, never any @ (ThinkFlowLab#1254) error: |- a pull link names its number, not undefined + '#undefined' - 'ThinkFlowLab#1234' not ok 4 - two dispatches on one issue through a qualified or URL id write one card (ThinkFlowLab#1254, ThinkFlowLab#1331) error: |- the same issue through a URL id reuses that card, so the id is the number 2 !== 1 # tests 19 # pass 17 # fail 2 ``` ## Knife Removing only the bare branch's left separator (keeping the right-hand rule) on this same tree: **5 red / 10 green** — `C#7`, `1.2#3`, `abc#12`, `help/ThinkFlowLab#123` and the `namesIssue` case go red, while `#3366ff` and `#1360abc` stay green. Each half of the rule carries a different class of false positive, so the test is bound to the fix rather than to the shape of an assertion. Restored: **15/15 green**. ## Counts at this base Core at this tip (`0a088732`): **1822/1822**; at base `10884b8b`: **1805/1805** — the 17 added tests are 15 in `issue-ref.test.js` and 2 in `work-dispatch.test.js`. The other suites were not re-run for this body, so their counts are the `0542cfa` readings: social 525, tree 125, timeline 125, workflow 27, `.github/scripts` 543, interop 37, live-e2e 24. ## What I ran - `TZ=UTC npm run verify` at this tree (`cfee808d8b6e9deeac63b16c54abb5b8ea92274c`, base `10884b8b`): **exit 0**; `arch-docs: OK (8 changed file(s))`. - `node core/scripts/build-client.mjs --check`: exit 0. - `node scripts/ds-bot-patches.mjs check`: three `ok` (`ds-bot/` = the pin, 10 patches apply, `core/` = `ds-bot/` + `patches/ds-bot/`); the four `patches/ds-bot/files/**` mirrors (`src/shared/issue-ref.js`, `src/host/work-dispatch.js`, `test/issue-ref.test.js`, `test/work-dispatch.test.js`) come from `generate`, never by hand. - The red/green pair above, and the knife. ## What I did not run - `core/dev/bench.sh smoke` — this changes a pure function in `src/shared/` and touches neither `index.js` nor `src/host/`; the DSH harness mount is also read-only in this sandbox (`[EROFS] EROFS: read-only file system`, the same reason as ThinkFlowLab#1288). - Live rooms and a real Bot round. Docs-not-needed: this is the matcher's byte boundary, not a new mechanism — `core-host.md` describes `issue-ref.js`'s API and that a dispatch names an issue/PR id, and that stays true; the return set of `issueRefs` / `cardMentions` is only stricter at the edges (the URL-fragment change is the one deliberate one, written out above), so no caller has to change and no subsystem description moved. `.github/**` is untouched. ## Checkpoint <!-- flybot:checkpoint --> ```json {"pr":1392,"claim":"Dev2","issue":1391,"summary":"`issue-ref.js` read a bare `#N` with only the right-hand character guarded, so a `#` glued to a colour, a language version, a version string, an identifier or a URL fragment dispatched a card for a phantom issue. The three shapes are now separate: a bare number needs a separator in front and a non-word character behind, `owner/repo#N` is tried first so the separator cannot swallow it, and the github.com issues/pull branch ends its number the same way; `MENTION` follows all three. A URL fragment (`help/ThinkFlowLab#123`) deliberately stops parsing.","files":["core/src/shared/issue-ref.js","core/src/host/work-dispatch.js","core/test/issue-ref.test.js","core/test/work-dispatch.test.js","patches/ds-bot/files/src/shared/issue-ref.js","patches/ds-bot/files/src/host/work-dispatch.js","patches/ds-bot/files/test/issue-ref.test.js","patches/ds-bot/files/test/work-dispatch.test.js"],"checks":{"ran":"TZ=UTC npm run verify; node core/scripts/build-client.mjs --check; node scripts/ds-bot-patches.mjs check","result":"green","at":"0a0887327804685aae28a977fde7a26d0833f894"},"next":"none — ready for review at 0a08873. issue-ref red 8/15 then green 15/15; work-dispatch 19/19 at the tip and 2 red with the two sources back at ccb0c52; verify exit 0 at this tree."} ``` <!-- /flybot:checkpoint -->
Written-by: Dev
Owner: Dev
Reviewer: Reviewer
Fixes #1109
What changed
Unfinished conversation tasks wake the owning Bot. After a board write, and again when a turn ends, the host starts a continue turn for that Bot's next task: its own chat, or its group Session for a subtask. The wake is a
[Continue task]follow-up, not a group post.Owner note: in a group, members with their own subtasks advance in parallel in the same drive window. One assignment wakes each of those members in its own Session. The work is not serialized onto one Bot or one turn. A single Bot still does one Next task at a time. The same Session is not woken again while its turn is open, and a task waiting on an answer does not wake in a loop. A live group round still defers member wakes until that round ends, so the driver does not add a turn beside a round.
Verification at the tip
Tip SHA:
83e41117e7139f5bcf585131ec03847005ffe31aRan:
TZ=UTC npm run verifyon Node v22.23.4. Lint, plugin lint,build-client.mjs --check,ds-bot-patches.mjs check, core tests 1468 pass, social 479, tree 125, timeline 125,.github/scripts417, interop 37, live-e2e 16. All fail 0.Not run: the smoke bench (
core/dev/bench.sh). This machine has no deepseek-harness checkout for the bench. The host behavior is covered bycore/test/task-drive.test.js.